Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix LIKWID_VERSION and LIKWID_COMMIT macros in likwid.h not being set correctly during installation #653

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

OoJJBoO
Copy link
Contributor

@OoJJBoO OoJJBoO commented Dec 3, 2024

While playing around with different versions, I noticed that the LIKWID_VERSION and LIKWID_COMMIT macros are currently not set correctly during the installation step, because there are some discrepancies between the naming used in likwid.h and the scheme the corresponding sed command in the Makefile expects.

Using the changes in this commit, the insertion should work as originally intended.

@TomTheBear
Copy link
Member

Thanks for the PR. It has to wait until the next major release as it changes the main header likwid.h.

@OoJJBoO
Copy link
Contributor Author

OoJJBoO commented Dec 3, 2024

I see, thanks anyway for the quick response :)

@OoJJBoO
Copy link
Contributor Author

OoJJBoO commented Dec 3, 2024

Alternatively, we could probably also do it the other way around and only adjust the expected names in the Makefile, right? Would that be better @TomTheBear ?

@TomTheBear
Copy link
Member

Changing the names in the Makefile is probably a better approach. As long as no change happens to likwid.h, it can be merged directly and can be included in 5.4.1.

@OoJJBoO OoJJBoO force-pushed the fix_likwid_h_version_macro branch from e3f2db6 to 8e5090f Compare December 3, 2024 16:41
@OoJJBoO
Copy link
Contributor Author

OoJJBoO commented Dec 3, 2024

Just replaced the first commit, this time only modifying the Makefile. Looks to also work just fine this way.

@TomTheBear TomTheBear merged commit 86f21f3 into RRZE-HPC:master Dec 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants